Skip to content

Cherry-pick JWT CVE fix into 1.4 (#12)#2667

Merged
istio-testing merged 1 commit intoistio:release-1.4from
howardjohn:14/cp-144
Feb 11, 2020
Merged

Cherry-pick JWT CVE fix into 1.4 (#12)#2667
istio-testing merged 1 commit intoistio:release-1.4from
howardjohn:14/cp-144

Conversation

@howardjohn
Copy link
Member

Problem: The JWT filter when matching exact paths included query parameters
which meant the JWT requirement could be bypassed by adding a "?" after the
path. The API was intended to only work for URIs.

Solution: The fix updates the match logic to only include URIs i.e. path
stripped off the query section.

Added unit tests to validate these cases.

  • Fixed formatting

  • Strip fragment of Path

Added unit tests to validate combination of query & fragment

(cherry picked from commit 859552a)

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

* Fixed JWT CVE related to exact PATH matches (istio#9)

* Fixed JWT CVE related to exact PATH matches

Problem: The JWT filter when matching exact paths included query parameters
which meant the JWT requirement could be bypassed by adding a "?" after the
path. The API was intended to only work for URIs.

Solution: The fix updates the match logic to only include URIs i.e. path
stripped off the query section.

Added unit tests to validate these cases.

* Fixed formatting

* Strip fragment of Path

Added unit tests to validate combination of query & fragment

* Fix lint

* Minor refactoring and more unit test cases (istio#11)

* Minor refactoring and more unit test cases

* Lint fixes

(cherry picked from commit 859552a)
@howardjohn howardjohn requested a review from a team February 11, 2020 18:44
@googlebot
Copy link
Collaborator

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. label Feb 11, 2020
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 11, 2020
@howardjohn howardjohn added cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels Feb 11, 2020
@googlebot
Copy link
Collaborator

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@istio-testing istio-testing merged commit 2369b2f into istio:release-1.4 Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants